Conversation
| } | ||
| catch (Exception e) | ||
| { | ||
| return name; // don't try to be clever if the name is not legal |
There was a problem hiding this comment.
If the name is not legal do we want to return it here or throw?
There was a problem hiding this comment.
The change is meant to skip the NetworkDrive.exists(f) check if the name is illegal. As for throwing, that would be a bigger change for when we enforce check more broadly.
There was a problem hiding this comment.
At least one of the callers for tryName also does a check for NetworkDrive.exists with the name that is returned here. Is that not a problem if the name has dots and slashes in it? Or am I misunderstanding the usages?
There was a problem hiding this comment.
Good point. I've updated the caller (FileType line 352) to use a different method to resolve file. Looking at the callers it seems Path.of(name) would actually never be called since parentDir is never null. So it seems pretty safe to throw. I've updated to throw instead.
There was a problem hiding this comment.
Might also need to update FileType.getPath, which also uses parentdir.resolve(getName(...)), which calls tryName. There are also a couple of other usages of FileType.getName that might need validating or updating.
Rationale
Related Pull Requests
Changes